gh-104683: Argument Clinic: Extract parse_function_names() helper#107964
gh-104683: Argument Clinic: Extract parse_function_names() helper#107964erlend-aasland merged 12 commits intopython:mainfrom
Conversation
Reduce duplicate code in state_modulename_name()
|
This refactor helped me discover a regression of gh-107885; Argument Clinic does not /*[clinic input]
class C "void *" ""
C.meth
a: int
[clinic start generated code]*/
/*[clinic input]
# This should not be allowed!
C.__new__ = C.meth
[clinic start generated code]*/
/*[clinic input]
# Neither should this
@classmethod
C.__init__ = C.meth
[clinic start generated code]*/We should bake those assertions into the |
|
Post this, I want to further extract helpers from this stage of the state machine; with this refactor, the remaining code is clearer (simply, since it is fewer lines of code), and it is now obvious we need to do something with the |
Off-topic, but I'm pretty sure this block of code is unreachable FYI: cpython/Tools/clinic/clinic.py Lines 2440 to 2441 in 13c36dc It's not currently covered by any of our tests. If our type annotations are correct, moreover (and I believe they are), |
|
There are some corner cases which I think should be fixed:
I suggest we create an issue for this, and retarget this PR as a fix for these issues. |
|
The knot of code in #107964 (comment) is such a complex conditional (and, it appears, apparently buggy as well) that I think I'd prefer to see a dedicated PR untangling just that bit, and fixing the bug, before we go ahead with a change that factors out the common logic into a helper function. Would that be okay? |
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
AlexWaygood
left a comment
There was a problem hiding this comment.
Oh, one optional other suggestion:
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
|
Thanks for the review! |
|
I'll follow-up with further refactorings in this part of the source code; there's still a lot of duplicated code there. |
Reduce duplicate code in state_modulename_name()
Tools/clinic/#104683